Update: enable A5 SDMA workspace by default by doraemonmj · Pull Request #1615 · hw-native-sys/simpler

doraemonmj · 2026-07-31T09:26:57Z

Summary

provision the pinned PTO-ISA SDMA workspace in every A5 onboard build
keep URMA as the explicit alternative for the single communication workspace
run the A5 SDMA completion demo by default and remove the obsolete overlay guide

Validation

python -m pytest tests/ut/py/test_runtime_builder.py -q (45 passed)
pre-commit hooks passed

coderabbitai · 2026-07-31T09:27:18Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c26a814-2941-4326-8bb6-6141fe00585c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

A5 onboard runtime builds now enable SDMA workspace provisioning by default. URMA remains an explicit alternative. PTO-ISA resolution and embedding are unconditional for A5, and tests, examples, and documentation reflect the new behavior.

Changes

A5 workspace backend

Layer / File(s)	Summary
Runtime PTO-ISA setup `simpler_setup/runtime_builder.py`, `simpler_setup/runtime_compiler.py`	A5 runtime setup always resolves PTO-ISA metadata. Host configuration forwards only the URMA workspace option.
Host workspace backend `src/a5/platform/onboard/host/CMakeLists.txt`, `src/a5/platform/onboard/host/comm_hccl.cpp`	The host runtime derives SDMA as the default backend, enables URMA through its explicit option, and always requires PTO-ISA headers.
Runtime builder validation `tests/ut/py/test_runtime_builder.py`	Tests now validate default A5 PTO-ISA provenance, cache stamping, host defines, and URMA pin resolution.
Documentation and demo behavior `docs/`, `examples/a5/tensormap_and_ringbuffer/`	Documentation and demos describe default SDMA behavior, optional URMA builds, revised skip conditions, and removal of the obsolete overlay document.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

[Code Health] Require upgraded a5 environment supporting sdma #1315: This PR implements the issue’s proposed A5 default-SDMA behavior and removes the related gating.

Possibly related PRs

hw-native-sys/simpler#1179: This PR changes the SDMA workspace overlay introduced there from opt-in to default-on.
hw-native-sys/simpler#1404: Both PRs modify A5 PTO-ISA embedding and validation in the runtime setup and host CMake.
hw-native-sys/simpler#1534: Both PRs modify the SDMA overlay documentation and communication-domain references.

Poem

A rabbit hops where SDMA runs,
With PTO-ISA beneath the sun.
URMA waits by choice, not chance,
While tests and docs join the dance.
The old overlay fades from sight—
Default paths now build just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main change: enabling the A5 SDMA workspace by default.
Description check	✅ Passed	The description directly explains the default SDMA workspace, URMA alternative, demo changes, documentation removal, and validation results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@examples/a5/tensormap_and_ringbuffer/sdma_async_completion_demo/README.md`:
- Around line 21-33: Gate the demo’s test requirements in README.md to the SDMA
backend, and update kernel_sdma_tget_async.cpp to validate that the workspace
backend is SDMA before calling SdmaTget. Preserve the existing A5 platform and
two-die requirements while preventing execution when the workspace is URMA.

In
`@examples/a5/tensormap_and_ringbuffer/urma_deferred_completion_demo/test_urma_deferred_completion_demo.py`:
- Line 217: Update the skip reason in the URMA deferred completion test to state
that SIMPLER_ENABLE_PTO_URMA_WORKSPACE=ON must be set both before rebuilding and
when running pytest, so the test-time environment requirement is explicit.

In `@simpler_setup/runtime_builder.py`:
- Around line 201-205: Update the `_build_cache_stamp` documentation in
`_resolve_build_pto_isa_commit` to state that both A2A3 and A5 onboard builds
include the PTO-ISA commit. In `src/a5/platform/onboard/host/CMakeLists.txt`
lines 126-131, replace the SDMA-specific cache explanation with wording that
also covers explicit URMA builds embedding PTO-ISA headers.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7060565c-d42f-489b-93a5-39294f9e730c

📥 Commits

Reviewing files that changed from the base of the PR and between 80aa287 and ebfdedd.

📒 Files selected for processing (16)

docs/README.md
docs/a5-sdma-overlay.md
docs/capability-survey.md
docs/comm-domain.md
examples/a5/tensormap_and_ringbuffer/README.md
examples/a5/tensormap_and_ringbuffer/sdma_async_completion_demo/README.md
examples/a5/tensormap_and_ringbuffer/sdma_async_completion_demo/kernels/aiv/kernel_sdma_tget_async.cpp
examples/a5/tensormap_and_ringbuffer/sdma_async_completion_demo/test_sdma_async_completion_demo.py
examples/a5/tensormap_and_ringbuffer/urma_deferred_completion_demo/README.md
examples/a5/tensormap_and_ringbuffer/urma_deferred_completion_demo/kernels/aiv/kernel_urma_tget_async.cpp
examples/a5/tensormap_and_ringbuffer/urma_deferred_completion_demo/test_urma_deferred_completion_demo.py
simpler_setup/runtime_builder.py
simpler_setup/runtime_compiler.py
src/a5/platform/onboard/host/CMakeLists.txt
src/a5/platform/onboard/host/comm_hccl.cpp
tests/ut/py/test_runtime_builder.py

💤 Files with no reviewable changes (3)

docs/a5-sdma-overlay.md
examples/a5/tensormap_and_ringbuffer/sdma_async_completion_demo/test_sdma_async_completion_demo.py
docs/README.md

Enable the A5 SDMA workspace in the onboard host runtime and retain URMA as an explicit alternative. Record and validate the pinned PTO-ISA revision for every A5 onboard build so cache invalidation and load-time checks cover the compiled workspace backend. Run the SDMA completion demo by default and update the A5 documentation for the new backend selection.

doraemonmj · 2026-08-03T09:09:43Z

All CodeRabbit suggestions have been implemented. Changes made:

✅ SDMA demo requirements now gated to SDMA backend - README updated with backend-specific test requirements and kernel validation
✅ URMA test skip reason updated - explicitly states environment variable must be set both at build and test time
✅ PTO-ISA cache documentation updated - now correctly states both A2A3 and A5 onboard builds include PTO-ISA commit

All three AI review suggestions have been successfully addressed in the current code changes.

coderabbitai Bot reviewed Jul 31, 2026

View reviewed changes

Comment thread examples/a5/tensormap_and_ringbuffer/sdma_async_completion_demo/README.md

Comment thread ...tensormap_and_ringbuffer/urma_deferred_completion_demo/test_urma_deferred_completion_demo.py Outdated

Comment thread simpler_setup/runtime_builder.py

doraemonmj force-pushed the hccl branch 3 times, most recently from a8ed0dc to ad3bf76 Compare August 3, 2026 05:13

doraemonmj added 3 commits August 2, 2026 22:13

Docs: add SDMA workspace init failure diagnostic comment

8d25212

test: verify SDMA env fix after service restart

69ba9f1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update: enable A5 SDMA workspace by default - #1615

Update: enable A5 SDMA workspace by default#1615
doraemonmj wants to merge 3 commits into
hw-native-sys:mainfrom
doraemonmj:hccl

doraemonmj commented Jul 31, 2026

Uh oh!

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doraemonmj commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

doraemonmj commented Jul 31, 2026

Summary

Validation

Uh oh!

coderabbitai Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doraemonmj commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading